Skip to content

[SPARK-29539][SQL] SHOW PARTITIONS should look up catalog/table like v2 commands#26198

Closed
huaxingao wants to merge 2 commits intoapache:masterfrom
huaxingao:spark-29539
Closed

[SPARK-29539][SQL] SHOW PARTITIONS should look up catalog/table like v2 commands#26198
huaxingao wants to merge 2 commits intoapache:masterfrom
huaxingao:spark-29539

Conversation

@huaxingao
Copy link
Contributor

@huaxingao huaxingao commented Oct 21, 2019

What changes were proposed in this pull request?

Add ShowPartitionsStatement and make SHOW PARTITIONS go through the same catalog/table resolution framework of v2 commands.

Why are the changes needed?

It's important to make all the commands have the same table resolution behavior, to avoid confusing end-users.

Does this PR introduce any user-facing change?

Yes. When running SHOW PARTITIONS, Spark fails the command if the current catalog is set to a v2 catalog, or the table name specified a v2 catalog.

How was this patch tested?

Unit tests.

@huaxingao
Copy link
Contributor Author

huaxingao commented Oct 21, 2019

@viirya
Could you please review? Thanks a lot!

viirya
viirya reviewed Oct 21, 2019
View reviewed changes
Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me. Looks like loadTable can provide partitioning info so V2 implementation can also be implemented now? @cloud-fan @rdblue

@rdblue
Copy link
Contributor

rdblue commented Oct 22, 2019

This looks fine to me. Looks like loadTable can provide partitioning info so V2 implementation can also be implemented now?

The partitioning provided by loadTable are the transforms and partition columns, not the actual table partitions. SHOW PARTITIONS can't be implemented until we have an API for getting partition metadata.

viirya
viirya reviewed Oct 22, 2019
View reviewed changes
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

test("SHOW PARTITIONS") {
val sql1 = "SHOW PARTITIONS t1"
val sql2 = "SHOW PARTITIONS db1.t1"
Copy link
Member

@viirya viirya Oct 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a case of a.b.c.

Copy link
Contributor Author

@huaxingao huaxingao Oct 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @viirya for your review. The comment has been addressed.

@SparkQA
Copy link

SparkQA commented Oct 22, 2019

Test build #112414 has finished for PR 26198 at commit 003ab44.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class ShowPartitionsStatement(tableName: Seq[String],

@SparkQA
Copy link

SparkQA commented Oct 22, 2019

Test build #112436 has finished for PR 26198 at commit 157b755.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

cloud-fan commented Oct 22, 2019

the changes LGTM, let's fix the conflict

@SparkQA
Copy link

SparkQA commented Oct 22, 2019

Test build #112478 has finished for PR 26198 at commit 248513c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@huaxingao
Copy link
Contributor Author

huaxingao commented Oct 22, 2019

Conflicts fixed @viirya

@viirya
Copy link
Member

viirya commented Oct 22, 2019

Thanks. Merging this to master!

@viirya viirya closed this in 3bf5355 Oct 22, 2019
@huaxingao
Copy link
Contributor Author

huaxingao commented Oct 22, 2019

Thanks all for the help!

@huaxingao huaxingao deleted the spark-29539 branch October 22, 2019 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@viirya viirya viirya approved these changes

Assignees

No one assigned

Labels

SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@huaxingao @rdblue @SparkQA @cloud-fan @viirya @dongjoon-hyun